Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Param for Max Retrial of Engine Cranking. #27926

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

loki077
Copy link
Contributor

@loki077 loki077 commented Aug 26, 2024

As we use a common battery for Avionics and Engine Cranking we would like to have an option to limit the number of retrials so that it doesn't drain that battery completely. Obvious choice is to separate the batteries but I still think there could be some use of this feature.

My Implementation is by adding Param MAX_RETRY which If set 0 then there is no limit to retrials. If set to a value greater than 0 then the engine will retry starting the engine this many times before giving up. Also the counter to track retrials resets to zero on disarm (could be done only on power cycle rather than disarm). If needed to be override can be done by setting the param to 0.

I would like to get more ideas on the method of its implementation. I have not yest tested this code.

Other ways to implement this is by just keep increasing the time between retrials.

@loki077
Copy link
Contributor Author

loki077 commented Aug 26, 2024

@peterbarker @tridge FYI

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Aug 26, 2024
Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs an autotest

libraries/AP_ICEngine/AP_ICEngine.cpp Outdated Show resolved Hide resolved
libraries/AP_ICEngine/AP_ICEngine.cpp Outdated Show resolved Hide resolved
libraries/AP_ICEngine/AP_ICEngine.cpp Outdated Show resolved Hide resolved
libraries/AP_ICEngine/AP_ICEngine.cpp Outdated Show resolved Hide resolved
@robertlong13 robertlong13 force-pushed the Max-engine-cranking-retrial branch 2 times, most recently from 751c759 to cc8154c Compare September 17, 2024 01:32
libraries/AP_ICEngine/AP_ICEngine.cpp Outdated Show resolved Hide resolved
libraries/AP_ICEngine/AP_ICEngine.cpp Show resolved Hide resolved
libraries/AP_ICEngine/AP_ICEngine.h Show resolved Hide resolved
libraries/AP_ICEngine/AP_ICEngine.cpp Show resolved Hide resolved
loki077 and others added 2 commits September 24, 2024 22:18
Added Param MAX_RETRY which If set 0 or less, then there is no limit to retrials. If set to a value greater than 0 then the engine will retry starting the engine this many times before giving up.
libraries/AP_ICEngine/AP_ICEngine.cpp Outdated Show resolved Hide resolved
libraries/AP_ICEngine/AP_ICEngine.cpp Outdated Show resolved Hide resolved
self.set_parameter("ICE_RPM_CHAN", 120) # Set to a non-existent sensor
self.set_rc(rc_engine_start_chan, 2000)
self.wait_statustext("Uncommanded engine stop")
self.wait_statustext("Starting engine")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_context, similarly elsewhere in here

Copy link
Collaborator

@robertlong13 robertlong13 Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have to be careful here. I don't want

Starting engine
Engine running
Uncommanded engine stop
... [never starts again]

to pass, but if I check context on these, that would pass. I need to enforce an order. I expect to see

Starting engine
Engine running
Uncommanded engine stop
Starting engine
Engine running
Uncommanded engine stop
[... and so on and so forth]

Copy link
Collaborator

@robertlong13 robertlong13 Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again today, I think the test is good like it is currently; I got myself a little confused on the call.

@peterbarker peterbarker merged commit e57994f into ArduPilot:master Sep 27, 2024
95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants